Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add function to get standard deviation (CBA_fnc_standardDeviation) #1407

Merged
merged 12 commits into from
Jan 20, 2021
Merged

Add function to get standard deviation (CBA_fnc_standardDeviation) #1407

merged 12 commits into from
Jan 20, 2021
+78 −1

Conversation

Kexanone
Copy link
Contributor

@Kexanone Kexanone commented Jan 11, 2021 •
edited
Loading

When merged this pull request will:

  • Implement CBA_fnc_standardDeviation

@commy2

This comment has been minimized.

Sign in to view
@commy2
Copy link
Contributor

commy2 commented Jan 12, 2021

This should have a unit test tbh.

@Kexanone
Copy link
Contributor Author

Should have made it as a draft 😅

@Kexanone
Copy link
Contributor Author

Not an expert on this, but is there any meaning to calling this with anything else other than 0 or 1 as second argument?

I took numpy.std as inspiration, where they also use ddof as optional parameter to handle the two cases.

@commy2
Copy link
Contributor

commy2 commented Jan 12, 2021

Should have made it as a draft 😅

No worries, just want to get it right.
Also, never understood the point of drafts. Just label your pr as such if it is wip.

@veteran29
Copy link
Member

Also, never understood the point of drafts. Just label your pr as such if it is wip.

Drafts are useful if you have auto-requests for reviews for PRs based on code ownership. Which is not a case in this repo.

@Kexanone Kexanone marked this pull request as draft January 13, 2021 19:31
@Kexanone Kexanone marked this pull request as draft January 13, 2021 19:31
@Kexanone Kexanone marked this pull request as ready for review January 14, 2021 22:13
@Kexanone
Copy link
Contributor Author

Side note: Test for arrays component fails because of other functions, but my part is fine.

@Kexanone Kexanone changed the title Implement standard deviation Implement standard deviation and fix array unit tests Jan 16, 2021
@Kexanone Kexanone changed the title Implement standard deviation and fix array unit tests Implement standard deviation and fix arrays unit tests Jan 16, 2021
@Kexanone

This comment has been minimized.

Sign in to view
@commy2
Copy link
Contributor

commy2 commented Jan 16, 2021

Really?! Remove tests in the same PR you add features?

@Kexanone
Copy link
Contributor Author

Kexanone commented Jan 16, 2021 •
edited
Loading

Really?! Remove tests in the same PR you add features?

How am I supposed to write and validate unit tests when the component is completely broken?

Edit: They should have been done when the actual changes were made, which apparently didn't happen.

@Kexanone
Copy link
Contributor Author

Kexanone commented Jan 16, 2021 •
edited
Loading

I removed it, but at least we know now that it works with the patch.
Edit: Moved the patch to #1408.

@Kexanone Kexanone changed the title Implement standard deviation and fix arrays unit tests Implement standard deviation Jan 16, 2021
@commy2 commy2 added this to the 3.16 milestone Jan 16, 2021
@commy2 commy2 added the Feature label Jan 16, 2021
@commy2
Copy link
Contributor

commy2 commented Jan 16, 2021 

execVM "\x\cba\addons\arrays\test_standardDeviation.sqf"
21:37:08 [CBA] (arrays) LOG: Testing CBA_fnc_standardDeviation
21:37:08 [CBA] (arrays) Test OK: (CBA_fnc_standardDeviation is defined) x\cba\addons\arrays\test_standardDeviation.sqf:15
21:37:08 [CBA] (arrays) Test OK: (_result isEqualTo _expected) x\cba\addons\arrays\test_standardDeviation.sqf:20
21:37:08 [CBA] (arrays) Test OK: (_result isEqualTo _expected) x\cba\addons\arrays\test_standardDeviation.sqf:25
21:37:08 [CBA] (arrays) Test OK: (_result isEqualTo _expected) x\cba\addons\arrays\test_standardDeviation.sqf:30

@PabstMirror PabstMirror changed the title Implement standard deviation Add function to get standard deviation (CBA_fnc_standardDeviation) Jan 20, 2021
@PabstMirror PabstMirror merged commit 41efca1 into CBATeam:master Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonpas jonpas jonpas approved these changes

@commy2 commy2 commy2 approved these changes

Assignees
No one assigned
Labels
Feature
Projects
None yet
Milestone
3.15.3
Development

Successfully merging this pull request may close these issues.
5 participants
@Kexanone @commy2 @veteran29 @jonpas @PabstMirror